Skip to content

fix(discord): reuse existing starter threads#190

Open
chenjian-agent wants to merge 2 commits intoopenabdev:mainfrom
chenjian-agent:fix/discord-starter-thread-reuse
Open

fix(discord): reuse existing starter threads#190
chenjian-agent wants to merge 2 commits intoopenabdev:mainfrom
chenjian-agent:fix/discord-starter-thread-reuse

Conversation

@chenjian-agent
Copy link
Copy Markdown
Contributor

Summary

Fix Discord message handling when a starter thread already exists for the triggering message.

This makes get_or_create_thread() resilient instead of treating Discord's existing-thread response as a fatal error.

Closes #189

Root cause

The previous implementation always called create_thread_from_message(...) for non-thread messages.

If Discord had already created a starter thread for that message, the API returned:

A thread has already been created for this message

OpenAB logged the error and returned early, so the prompt was dropped even though a valid thread already existed.

Changes

  • import MessageFlags
  • reuse msg.thread when Discord includes the starter thread on the message payload
  • reuse msg.id when MessageFlags::HAS_THREAD is present
  • treat Discord's A thread has already been created for this message response as a recoverable case and fall back to the existing starter thread id

Validation

  • reproduced the production failure from logs on 2026-04-10
  • built locally with cargo build
  • deployed the patched binary on the production host
  • restarted openab.service
  • confirmed the service came back healthy and connected after restart

Why this approach

For message-based starter threads, Discord uses the starter message id as the thread id. That means the correct recovery path is to reuse the existing thread instead of failing the request.

@chenjian-agent
Copy link
Copy Markdown
Contributor Author

Production debugging notes for this fix:

  1. We first verified that the service itself was not down. openab.service remained active (running) and the bot was still connected to Discord.
  2. We checked the runtime logs and found repeated failures of the form:
    • failed to create thread: A thread has already been created for this message
  3. We traced the failure path to get_or_create_thread() in src/discord.rs.
  4. The existing logic always attempted create_thread_from_message(...) for non-thread messages and returned early on that Discord error.
  5. We confirmed that this made the bot drop otherwise valid requests whenever the triggering message already had a starter thread.
  6. We patched the handler to reuse the existing thread instead of failing:
    • return the current channel id when already inside a thread
    • reuse msg.thread when Discord includes the starter thread on the message payload
    • reuse msg.id when MessageFlags::HAS_THREAD is present
    • treat A thread has already been created for this message as a recoverable case and fall back to the existing starter thread id
  7. We validated the code with a local cargo build.
  8. We then deployed the patched binary on the production host and restarted openab.service to restore service.

This PR is the upstreamed version of that production fix.

@thepagent thepagent added the p2 Medium — planned work label Apr 15, 2026
@masami-agent
Copy link
Copy Markdown
Contributor

Review — PR #190

The idea is correct — reusing existing threads instead of failing on duplicate creation. The three-layer fallback (msg.threadHAS_THREAD flag → error string match) is a solid approach.

✅ What looks good

  1. Three-layer thread detection — checks msg.thread, then HAS_THREAD flag, then catches the Discord error. Covers all edge cases.
  2. msg.id as thread ID — correct, Discord uses the starter message ID as the thread ID for message-based threads.

🔴 Must fix before review can proceed

Base is significantly outdated. main has been refactored since this PR was opened:

  • get_or_create_thread now takes an adapter: &Arc<dyn ChatAdapter> parameter and returns ChannelRef instead of u64
  • main already has partial thread reuse via thread_metadata check (but does not cover the starter-thread case this PR addresses)
  • Handler struct has been restructured (new fields: router, adapter, allow_bot_messages, trusted_bot_ids)
  • Thread creation is now delegated to adapter.create_thread()

Please rebase onto latest main. The thread reuse logic will need to be adapted to the new ChatAdapter architecture.

🟡 Non-blocking

String matching on error messageerr.to_string().contains(THREAD_EXISTS_MSG) is fragile. If Discord changes the error text, this breaks silently. Acceptable for now since serenity doesn't expose a typed error for this case, but worth noting.

Summary

Valuable fix that complements the existing thread_metadata check on main. Rebase needed before we can proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Discord handler drops requests when a starter thread already exists

4 participants